Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Defer metadata target import #77

Merged
merged 13 commits into from
Jul 5, 2023
Merged

Conversation

cisaacstern
Copy link
Member

@cisaacstern cisaacstern commented Jul 4, 2023

Fixes #76
Also fixes #58, which was necessary to adequately test this.

@cisaacstern cisaacstern added the test-dataflow Add this label to PRs to trigger Dataflow integration test. label Jul 4, 2023
@codecov
Copy link

codecov bot commented Jul 4, 2023

Codecov Report

Merging #77 (e84eac2) into main (3b71752) will increase coverage by 3.34%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #77      +/-   ##
==========================================
+ Coverage   82.96%   86.30%   +3.34%     
==========================================
  Files          13       13              
  Lines         405      409       +4     
==========================================
+ Hits          336      353      +17     
+ Misses         69       56      -13     
Impacted Files Coverage Δ
pangeo_forge_runner/bakery/dataflow.py 97.61% <ø> (ø)
pangeo_forge_runner/commands/bake.py 93.25% <100.00%> (+2.56%) ⬆️
pangeo_forge_runner/storage.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@cisaacstern
Copy link
Member Author

All tests are passing with exception of Flink test (which is known to be broken), so I'm going to merge.

I was having a fair bit of trouble with the internal beam subprocess call here not finding a functional version of pip. But ended up being able to side-step that problem by adding the --sdk_location="container" option to our Dataflow pipeline options, which turns out is recommended when using a container anyway.

@cisaacstern cisaacstern merged commit 7650398 into main Jul 5, 2023
6 of 7 checks passed
@yuvipanda yuvipanda deleted the defer-metadata-target-import branch November 3, 2023 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test-dataflow Add this label to PRs to trigger Dataflow integration test.
Projects
None yet
1 participant